Skip to content

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Jul 31, 2025

User description

🔗 Related Issues

#16113

💥 What does this PR do?

This PR updates Python packaging:

  • fixes license metadata in py/pyproject.toml to conform to PEP 639: https://peps.python.org/pep-0639 (the current way we define license metadata is deprecated and will no longer work after February 2026)
  • updates Bazel configuration to include copyright notice file (NOTICE) in the packages we build for distribution (wheel/sdist)

🔄 Types of changes

  • Build/Packaging

PR Type

Other


Description

  • Update Python license metadata to PEP 639 compliance

  • Include NOTICE file in distribution packages

  • Add Bazel rules for copyright notice handling

  • Create symlinks for LICENSE and NOTICE files


Diagram Walkthrough

flowchart LR
  A["License Metadata"] --> B["PEP 639 Compliance"]
  C["NOTICE File"] --> D["Distribution Package"]
  E["Bazel Rules"] --> F["Copyright Handling"]
  B --> G["Updated pyproject.toml"]
  D --> G
  F --> G
Loading

File Walkthrough

Relevant files
Configuration changes
BUILD.bazel
Add NOTICE file handling to Bazel build                                   

py/BUILD.bazel

  • Add Bazel rules for NOTICE file handling
  • Include NOTICE file in wheel and sdist packages
  • Update license string format in py_wheel rule
+21/-1   
LICENSE
Add LICENSE symlink                                                                           

py/LICENSE

  • Create symlink to root LICENSE file
+1/-0     
NOTICE
Add NOTICE symlink                                                                             

py/NOTICE

  • Create symlink to root NOTICE file
+1/-0     
pyproject.toml
Update license metadata for PEP 639                                           

py/pyproject.toml

  • Update license format to PEP 639 standard
  • Add license-files array with LICENSE and NOTICE
  • Include NOTICE in package data
+4/-2     

@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels Jul 31, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Jul 31, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Jul 31, 2025

PR Code Suggestions ✨

Latest suggestions up to fdcddcd

CategorySuggestion                                                                                                                                    Impact
Incremental [*]
Include NOTICE in license-files array

Consider adding the NOTICE file to the license-files array as well, since it
contains copyright notices that should be distributed with the package metadata

py/pyproject.toml [71-72]

+"LICENSE",
+"NOTICE",
 
-
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the NOTICE file should be added to the license-files key in pyproject.toml for packaging correctness, which was missed in the PR.

Medium
  • Update

Previous suggestions

✅ Suggestions up to commit 1eac865
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix missing comma in array
Suggestion Impact:The suggestion was directly implemented - a comma was added after "LICENSE" and also after "NOTICE" to properly format the TOML array

code diff:

-    "LICENSE"
-    "NOTICE"
+    "LICENSE",
+    "NOTICE",

Add a missing comma after "LICENSE" to properly separate the list items. Without
the comma, this creates a syntax error in the TOML array.

py/pyproject.toml [71-72]

-"LICENSE"
+"LICENSE",
 "NOTICE"

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a missing comma in the TOML array, which is a syntax error that would cause the package build to fail.

High

@cgoldberg cgoldberg marked this pull request as draft August 1, 2025 18:19
@cgoldberg cgoldberg marked this pull request as ready for review August 4, 2025 12:14
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 4, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

License Format

The license field format change from object to string should be validated to ensure it properly conforms to PEP 639 and works with all packaging tools

license = "Apache-2.0"
license-files = ["LICENSE", "NOTICE"]
File Dependencies

The new NOTICE file handling assumes the NOTICE file exists in the root license target. Should verify this dependency is correctly resolved

select_file(
    name = "global-notice",
    srcs = "//:license",
    subpath = "NOTICE",
)

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 4, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@cgoldberg
Copy link
Member Author

this seems to work.. I'm merging it, and will validate the package published to TestPyPI tomorrow.

@cgoldberg cgoldberg merged commit 2a74c64 into SeleniumHQ:trunk Aug 4, 2025
15 of 16 checks passed
@cgoldberg cgoldberg deleted the py-package-licensing branch August 4, 2025 12:16
@cgoldberg
Copy link
Member Author

packages looks good on TestPyPI and now include NOTICE file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-py Python Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants